-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Crypto: Add Java Cryptographic Analysis Queries #20605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…der-java-crypto-check
… used. Prefer 'type'.
…dards. Remove redundant query. Fix QL-for-QL issues.
This PR is based on #20568, made a copy to be able to do my own quick edits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds comprehensive Java cryptographic analysis queries to detect various security vulnerabilities and weak cryptographic practices. The changes include 11 new CodeQL analysis queries and updates to the underlying cryptographic models to improve consistency in naming conventions.
- Adds 11 new analysis queries to detect weak cryptographic implementations including weak symmetric/asymmetric ciphers, poor block modes, weak hashing algorithms, and insecure key derivation functions
- Updates cryptographic model naming from "Family" to "Type" for better consistency across elliptic curve and hash algorithm classifications
- Modifies precision levels for some existing queries to improve analysis accuracy
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
shared/quantum/codeql/quantum/experimental/Standardization.qll |
Renames elliptic curve type definitions from "Family" to "Type" |
shared/quantum/codeql/quantum/experimental/Model.qll |
Updates method names and documentation to use "Type" instead of "Family" |
java/ql/src/experimental/quantum/Analysis/*.ql |
Adds 11 new analysis queries for detecting cryptographic vulnerabilities |
java/ql/lib/experimental/quantum/JCA.qll |
Updates method implementations to use new "Type" naming convention |
cpp/ql/lib/experimental/quantum/OpenSSL/*.qll |
Updates C++ OpenSSL bindings to match new naming conventions |
@nicolaswill this was meant to be a draft PR while I fix up the last issues, sorry about that. I don't see how I can move it back to draft, so I'll leave this open but ping when it is ready for final review. |
…mpacts the insecure IV/Nonce query. Updated name of the Insecure nonce query to be InsecureIVorNonce
…al sources to include array literals.
… of JCA random source.
…o fix false positives in the unknown IV/Nonce query). Add the unknown IV/Nonce query and associated test cases. Fix unknown IV/Nonce query to focus on cases where the oepration isn't known or the operation subtype is not encrypt or wrap.
…wn for it, and if so do not alert on non-secure random if it is tied to decryption
…Added unknown hash.
I'm hesitant to merge this into the out-of-box queries, as many of these are prescriptive about allowed/disallowed algorithms or thresholds for things like iteration count. I would suggest putting these into an Examples directory. |
…des/codeql into santander-java-crypto-check
… for consistency.
…s' subdirectories.
… unknowns but where the algorithm category (e.g., hashing) is known.
...l/query-tests/quantum/examples/WeakOrUnknownAsymmetricKeySize/WeakAsymmetricKeyGenSize.qlref
Fixed
Show fixed
Hide fixed
…k and unknown KDF iteration count.
…xt (barriers were blocking flow through an encrypt to a subsequent mac on the same plaintext)
…at traces through any intermediate encrypt or mac to the final encrypt or mac.
…count for missing cipher algorithms.
…ID, to make clear the query is not intended for production.
…with ql-for-ql, use formatted test for WeakAsymmetricKeyGenSize (add post processing in the qlref), misc expected files updated (test passed locally but on rerun vscode reports failures, known bug with vscode unit tests).
…des/codeql into santander-java-crypto-check
result = this.(KnownMacAlgorithm).getConsumer() | ||
} | ||
|
||
override int getKeySizeFixed() { |
Check warning
Code scanning / CodeQL
Redundant override Warning
this predicate
result = super.getKeySizeFixed() | ||
} | ||
|
||
override Crypto::ConsumerInputDataFlowNode getKeySizeConsumer() { |
Check warning
Code scanning / CodeQL
Redundant override Warning
this predicate
result = super.getKeySizeConsumer() | ||
} | ||
|
||
override string getRawAlgorithmName() { |
Check warning
Code scanning / CodeQL
Redundant override Warning
this predicate
…assessments, altering a few queries weak/vuln/bad crypto to have qll files that can be accessed for other purposes, like graph generation. Also altering weak symmetric cipher to look for non-aes algorithms to be more comprehensive.
*/ | ||
|
||
import java | ||
import experimental.quantum.Language |
Check warning
Code scanning / CodeQL
Redundant import Warning
BadMacOrder
*/ | ||
|
||
import java | ||
import experimental.quantum.Language |
Check warning
Code scanning / CodeQL
Redundant import Warning
BadMacOrder
…es in an ArtifactNode to be used for graph generation to intercept nodes with known mac ordering issues, in order to format the node and output error messages in the graph.
} | ||
|
||
predicate isDecryptToMacNode(Crypto::ArtifactNode node) { | ||
exists(ArtifactFlow::PathNode src, ArtifactFlow::PathNode sink | |
Check warning
Code scanning / CodeQL
Omittable 'exists' variable Warning
in this argument
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are some example queries that check the cryptography present in output from a java source repo. Again, these build on the existing examples both in java and in other CBOM and cryptographic issue checking codeQL queries:
InsecureNonceGeneration.ql - as before
InsecureNonceSource.ql - as before
KnownWeakKDFIterationCount.ql - as before
NonAESGCMCipher.ql - detects non-AES in GCM mode ciphers. Can be updated to be 'non AES256 in GCM mode' but this gives more alerts on inferred key lengths.
ReusedNonce.ql - as before
UnknownKDFIterationCount.ql - as before
WeakAsymmetric.ql - finds weak asymmetric RSA ciphers using key lengths < 2048
WeakBlockModes.ql - similar to NonAESGCM, this finds instances of known-bad block modes ECB, CFB, OFB, and CTR
WeakHashing.ql - finds potentially weak hashing instances using the whitelist of SHA256, SHA384, and SHA512 (though this is yet to be checked against SHA3 variants)
WeakKDFIterationCount.ql - as before
WeakKDFKeySize.ql - as before
WeakRSA.ql - an allternative method from WeakAsymmetric.ql, but functionally the same.
WeakSymmetricCiphers.ql - detects known-weak ciphers from a blocklist of DES, TripleDES, DoubleDES, RC2, RC4, IDEA, and Blowfish.